Add initial CoreCLR compilation support for Apple Silicon #40435

sdmaclea · 2020-08-06T01:23:17Z

Add initial CoreCLR compilation support for Apple Silicon

Initial draft of CoreCLR runtime native code to support Apple Silicon

Fixes native runtime compilation issues
Draft implementation
Testing limited to compilation on Apple

Does not fix Arm64 ABI issues
~~Does not~~ Prototypes fix write no execute issues

Initial draft of CoreCLR runtime native code to supprt Apple Silicon Fixes native runtime compitation issues Draft implementation Testing linited to compilation on Apple Does not fix Arm64 ABI issues Does not fix write no execute issues

sdmaclea · 2020-08-06T01:28:10Z

Status:

src/coreclr/build-runtime.sh succeeds on MacOS-arm64
This shouldn't merge with master branch due to risk to 5.0. (This is the reason the PR is marked draft).
Need to run CI to see if this causes 5.0 regression.

I would appreciate eyes on this. It hasn't been tested yet, but anything we find in code review will simplify bring-up.

/cc @tommcdon @JulieLeeMSFT @mangod9 @noahfalk

I think this unblocks the JIT team.

src/coreclr/clrdefinitions.cmake

sdmaclea · 2020-08-06T01:35:22Z

src/coreclr/src/pal/inc/unixasmmacrosarm64.inc

-        .long 0x04000000 // DWARF
-        .quad 0
-        .quad 0
-#endif


This code wasn't quite right. The value for arm64 dwarf is 0x03000000. However even with that fixed the linker was reporting compact unwind errors.

The comments in the Apple compact unwind header suggests the linker will no longer attempt to create compact unwind entries from dwarf tables. Specifically because they found it was too fragile.

I chose to remove this so that the linker was forced to use the dwarf unwind info for these hand coded assembly files.

w/o this change the linker reportes errors related to overlapping __compact_unwind entries as well as corrupt size.

IMHO this seems reasonable....

mangod9 · 2020-08-06T01:41:20Z

nice progress @sdmaclea. Great that things are now building.

src/coreclr/src/pal/src/exception/remote-unwind.cpp

src/coreclr/src/pal/inc/pal.h

src/coreclr/src/pal/src/thread/context.cpp

src/coreclr/src/pal/src/misc/sysinfo.cpp

src/coreclr/src/pal/src/exception/remote-unwind.cpp

src/coreclr/src/pal/src/exception/machexception.cpp

sdmaclea · 2020-09-29T21:51:14Z

I have started a new Priority1 test run on Apple Silicon using tests and runtime built on osx-arm64. It looks like the cross built code is working properly. Thanks @janvorli.

This should be really close to passing CI. We should be able to merge this soon.

The W^X code is known to be preliminary. We intend to refactor in a later PR for

W^X support on other platforms
Better locality. Keep the enable of writing closer to the code which does the writing.

@janvorli Can you please take another look?

eng/native/configureplatform.cmake

src/coreclr/src/pal/src/libunwind_mac/src/missing-functions.c

eng/build.sh

janvorli · 2020-10-01T10:57:13Z

src/coreclr/src/pal/src/cruntime/printfcpp.cpp

    if(convert)
    {
        int nsize;
        LPSTR newBuff = 0;
        nsize = WideCharToMultiByte(CP_ACP, 0,(LPCWSTR)buffer, count, 0, 0, 0, 0);
        if (!nsize)
        {
+            if (count == 0)


I still wonder why this occurs for arm64 OSX only and it was fine before.

I don't remember the test which caught this issue. I looked at the call stack briefly and it looked like a bogus failure to me. This looked like the right fix.

I opened #42954. To investigate this further when we have more capacity for testing on Apple Silicon.

janvorli · 2020-10-01T11:16:37Z

src/coreclr/src/pal/src/exception/machexception.cpp

-        // TBD // Inject the activation only if the thread doesn't have a pending hardware exception
+        // Inject the activation only if the last ESR.EC was an SVC and therefore the thread doesn't have
+        // a pending hardware exception
+        if ((ExceptionState.__esr >> 26) == 0x15)


Can you please make these named constants or defines to make it clear?
E.g. ESR_EC_SHIFT for the 26 and EC_SVC for 0x15 or something like that.

src/coreclr/src/vm/arm64/stubs.cpp

janvorli · 2020-10-01T11:59:23Z

src/coreclr/src/vm/exceptionhandling.cpp

@@ -3312,6 +3312,10 @@ DWORD_PTR ExceptionTracker::CallHandler(
        break;
    }



It is not clear to me why we need it here.

I would guess it is related to the MakeCallbacksRelatedToHandler() call below which can call managed code. I am not sure why I didn't place it above the MakeCallbacksRelatedToHandler() call above, but perhaps we simply didn't have coverage of that case.

It is probably not the right place to put this ultimately, but it is related to the arbitrary nature of the current PAL_JITWriteEnable(*) placement.

I would suggest leaving this to be cleaned up by #41124

janvorli · 2020-10-01T12:00:14Z

src/coreclr/src/vm/exceptionhandling.cpp

@@ -3948,6 +3952,10 @@ void ExceptionTracker::ResumeExecution(
    EH_LOG((LL_INFO100, "resuming execution at 0x%p\n", GetIP(pContextRecord)));
    EH_LOG((LL_INFO100, "^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n"));

+#if defined(HOST_OSX) && defined(HOST_ARM64)


And I would like to understand why we need it here either.

I think this might have been added to fix a case where we were trying to JIT a method which didn't exist (probably a dynamic method builder test). Effectively the pre*stub threw an exception and the exception handler required us to return to managed code in the managed exception handler.

So this too is introduced by the ad-hoc placement of the PAL_JITWriteEnable(*) calls.

I would suggest leaving this to be cleaned up by #41124.

src/coreclr/src/vm/gcinfodecoder.cpp

src/libraries/Native/Unix/System.Security.Cryptography.Native.Apple/pal_ssl.c

sdmaclea · 2020-10-01T21:44:51Z

@janvorli I believe I have addressed your comments either directly or by opening issues to be solved later.

sdmaclea · 2020-10-06T18:09:57Z

@janvorli The previous commit was green. The most recent commit doesn't touch code currently compiled in CI.
Please take a look again. I would like to merge this as is and iterate in future PRs.

janvorli

The current state LGTM modulo the two comments. But I'd like to get a review of @jkotas now that the change is final.

src/coreclr/src/pal/src/exception/machexception.cpp

eng/native/configureplatform.cmake

jkotas · 2020-10-06T21:42:01Z

src/coreclr/src/gc/unix/configure.cmake

+int main()
+{
+  int ret;
+  ret = clock_gettime_nsec_np(CLOCK_UPTIME_RAW);


Just curious - what was wrong with mach_absolute_time ?

It doesn't exist on aarch64 and Apple doc for mach_absolute_time recommends to replace it with clock_gettime_nsec_np

See https://developer.apple.com/documentation/kernel/1462446-mach_absolute_time

I have noticed that HAVE_MACH_ABSOLUTE_TIME is still used under libraries. Do we need to fix it there as well?

Hmm, the strange thing is that libraries compile for arm64 with it just fine. So maybe the failure due to which I've modified the usage of mach_absolute_time to clock_gettime_nsec_np was something temporary in the middle of the fixes I was making. Anyways, the change is good since the mach_absolute_time is deprecated, so we should change it in the libraries too.

Ok, opened #43320

src/coreclr/src/gc/unix/gcenv.unix.cpp

src/coreclr/src/pal/src/misc/sysinfo.cpp

jkotas

LGMT

sdmaclea added 3 commits August 5, 2020 17:49

Remove compact unwind entries from assembly files

83a45b8

Disable FEATURE_WRITEBARRIER_COPY

af1acc4

sdmaclea added arch-arm64 area-PAL-coreclr port os-macos-bigsur (macOS11) labels Aug 6, 2020

sdmaclea added this to the 6.0.0 milestone Aug 6, 2020

sdmaclea requested review from jkotas, mikem8361, janvorli, AntonLapounov and sandreenko August 6, 2020 01:23

sdmaclea self-assigned this Aug 6, 2020

sdmaclea marked this pull request as draft August 6, 2020 01:23

sdmaclea commented Aug 6, 2020

View reviewed changes

src/coreclr/clrdefinitions.cmake Outdated Show resolved Hide resolved

sdmaclea commented Aug 6, 2020

View reviewed changes

mangod9 added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Aug 6, 2020

mangod9 reviewed Aug 6, 2020

View reviewed changes

src/coreclr/src/pal/src/exception/remote-unwind.cpp Outdated Show resolved Hide resolved

jkotas reviewed Aug 6, 2020

View reviewed changes

src/coreclr/src/pal/inc/pal.h Outdated Show resolved Hide resolved

CONTEXT_SEGMENTS feedback

37d29f8

sdmaclea commented Aug 6, 2020

View reviewed changes

src/coreclr/src/pal/src/thread/context.cpp Outdated Show resolved Hide resolved

sdmaclea commented Aug 6, 2020

View reviewed changes

src/coreclr/src/pal/src/misc/sysinfo.cpp Outdated Show resolved Hide resolved

sdmaclea commented Aug 6, 2020

View reviewed changes

src/coreclr/src/pal/src/exception/remote-unwind.cpp Outdated Show resolved Hide resolved

sdmaclea commented Aug 6, 2020

View reviewed changes

src/coreclr/src/pal/src/exception/remote-unwind.cpp Outdated Show resolved Hide resolved

sdmaclea commented Aug 6, 2020

View reviewed changes

src/coreclr/src/pal/src/exception/machexception.cpp Show resolved Hide resolved

sdmaclea added 2 commits August 6, 2020 11:41

Revert incidental change

8d21b27

Revise StepWithCompactEncoding

e72e45c

sdmaclea added 2 commits September 29, 2020 15:03

Fix Windows build

fcf3424

Fix arm64 crossdac build

e186f01

This was referenced Sep 30, 2020

Mono System.Text.Json.Tests on Windows timing out #42677

Open

Builds legs getting abandoned due to agent notification issues #35223

Closed

Mono SIGABRT in System.Drawing.Common affecting some test runs #37838

Closed

janvorli reviewed Oct 1, 2020

View reviewed changes

sdmaclea added 2 commits October 1, 2020 15:21

Merge remote-tracking branch 'origin/master' into dev/Arm64MacOS

6817c64

Review feedback

f9b9fff

Add missing semicolon

d3e49f0

sdmaclea mentioned this pull request Oct 6, 2020

Investigate need for Apple Silicon fix to Internal_Convertfwrite(...) #42954

Closed

janvorli reviewed Oct 6, 2020

View reviewed changes

src/coreclr/src/pal/src/exception/machexception.cpp Outdated Show resolved Hide resolved

jkotas reviewed Oct 6, 2020

View reviewed changes

eng/native/configureplatform.cmake Show resolved Hide resolved

jkotas reviewed Oct 6, 2020

View reviewed changes

janvorli closed this Oct 6, 2020

janvorli reopened this Oct 6, 2020

jkotas reviewed Oct 7, 2020

View reviewed changes

src/coreclr/src/gc/unix/gcenv.unix.cpp Outdated Show resolved Hide resolved

jkotas reviewed Oct 7, 2020

View reviewed changes

src/coreclr/src/pal/src/misc/sysinfo.cpp Outdated Show resolved Hide resolved

jkotas approved these changes Oct 7, 2020

View reviewed changes

PR Feedback

0d4eba8

janvorli approved these changes Oct 7, 2020

View reviewed changes

sdmaclea merged commit fd094a9 into dotnet:master Oct 8, 2020

runfoapp bot mentioned this pull request Oct 8, 2020

System.IO.Tests.File_GetSetTimes.SetDateTimeMax fails on Ubuntu 18.04 in the CI #43166

Closed

sdmaclea mentioned this pull request Oct 9, 2020

Port cross oxs-arm64 build support from runtime dotnet/diagnostics#1642

Merged

jkotas mentioned this pull request Oct 12, 2020

Change libraries to use clock_gettime_nsec_np instead of mach_absolute_time #43320

Closed

sdmaclea mentioned this pull request Nov 20, 2020

Epic: Support Apple Silicon #43313

Closed

31 tasks

ghost locked as resolved and limited conversation to collaborators Dec 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial CoreCLR compilation support for Apple Silicon #40435

Add initial CoreCLR compilation support for Apple Silicon #40435

sdmaclea commented Aug 6, 2020 •

edited

Loading

sdmaclea commented Aug 6, 2020 •

edited

Loading

sdmaclea Aug 6, 2020

mangod9 commented Aug 6, 2020

sdmaclea commented Sep 29, 2020

janvorli Oct 1, 2020

sdmaclea Oct 1, 2020

janvorli Oct 1, 2020

janvorli Oct 1, 2020

sdmaclea Oct 1, 2020

janvorli Oct 1, 2020

sdmaclea Oct 1, 2020

sdmaclea commented Oct 1, 2020

sdmaclea commented Oct 6, 2020

janvorli left a comment

jkotas Oct 6, 2020

janvorli Oct 6, 2020

janvorli Oct 6, 2020

jkotas Oct 11, 2020

janvorli Oct 12, 2020

jkotas Oct 12, 2020

jkotas left a comment

		@@ -3312,6 +3312,10 @@ DWORD_PTR ExceptionTracker::CallHandler(
		break;
		}

Add initial CoreCLR compilation support for Apple Silicon #40435

Add initial CoreCLR compilation support for Apple Silicon #40435

Conversation

sdmaclea commented Aug 6, 2020 • edited Loading

sdmaclea commented Aug 6, 2020 • edited Loading

Choose a reason for hiding this comment

mangod9 commented Aug 6, 2020

sdmaclea commented Sep 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdmaclea commented Oct 1, 2020

sdmaclea commented Oct 6, 2020

janvorli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkotas left a comment

Choose a reason for hiding this comment

sdmaclea commented Aug 6, 2020 •

edited

Loading

sdmaclea commented Aug 6, 2020 •

edited

Loading